Skip to content

feat: resolve project-id and org-id using instance-id#38

Draft
LucDeCaf wants to merge 1 commit intomainfrom
feat/optional-id-params
Draft

feat: resolve project-id and org-id using instance-id#38
LucDeCaf wants to merge 1 commit intomainfrom
feat/optional-id-params

Conversation

@LucDeCaf
Copy link
Copy Markdown

Replaces #37 using a cleaner and (hopefully) simpler approach.


Using a soon-to-be-introduced path in @powersync/management-client, automatically fetch the project_id and org_id flags when given the instance_id for a number of commands (where it makes sense to do so).

Example

Before:

powersync pull instance --project-id='project-123-456-789' --instance-id='instance-123-456-789'

After:

# Project ID is fetched from the management service if required
powersync pull instance --instance-id='instance-123-456-789'

NB: Test failures are expected until the next version of the @powersync/management-client releases.


function ensureObjectId(value: string, flagName: '--instance-id' | '--org-id' | '--project-id') {
function ensureObjectId(value: string | undefined, flagName: '--instance-id' | '--org-id' | '--project-id') {
if (value == null) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Under what cases would value be undefined or null? It seems like we would not reach this case in the current flow.

projectId?: string;
};

function ensureObjectId(value: string | undefined, label: '--instance-id' | '--org-id' | '--project-id'): void {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: It could be nice if we could export this function from the core package, instead of duplicating it here and in the cli's main.

return { instanceConfig, linked };
}

if (!orgId || !projectId) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic here is becoming a bit more difficult to follow. Would it be possible to (pre)populate the orgId and projectId if only the instanceId is provided? Then, I would assume the logic of this function could remain as it was.

Otherwise, maybe lets add some code comments for the various paths.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants